This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
Improve bulking in Gluon #13890
Merged
Improve bulking in Gluon #13890
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Perf comparison:
|
|
Also adding @KellenSunderland |
|
Thanks for your contribution @ptrendx |
|
The issue with test_dropout is unrelated, details in #9816 |
eric-haibin-lin
approved these changes
Jan 24, 2019
There was a problem hiding this comment.
Looks good to me. @piiswrong Could you double check this PR?
stephenrawls
pushed a commit
to stephenrawls/incubator-mxnet
that referenced
this pull request
Feb 16, 2019
* Improve bulking in Gluon * Trigger CI
7 tasks
vdantu
pushed a commit
to vdantu/incubator-mxnet
that referenced
this pull request
Mar 31, 2019
* Improve bulking in Gluon * Trigger CI
3 tasks
haohuanw
pushed a commit
to haohuanw/incubator-mxnet
that referenced
this pull request
Jun 23, 2019
* Improve bulking in Gluon * Trigger CI
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR improves performance of hybridized Gluon models on the GPU by better bulking of ops (running GPU ops without synchronization).
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
hybridize()engine currently createsImperativeBulkop that combines multiple operations into 1. However, thisImperativeBulkop captures entire ops, including synchronization at the end. Since the engine dependencies are only updated at the very end of theImperativeBulkop, having this synchronization is wasteful. This PR changes the RunContext structure so thatImperativeBulkop is able to inform the bulked ops that it will handle the synchronization itself. Imperative ops check that argument to determine whether they need to perform their own synchronization.hybridize(static_alloc=True, static_Shape=True)there is currently another mechanism of bulking, similar to the one used in symbolic API. However, it very aggressively excludes ops from being eligible for bulking - any op that produces output from forward pass (which means most of the ops) cannot be bulked. This is artificial limitation and this PR lifts it.Comments
hybridize(static_alloc=True)andhybridize(static_alloc=True, static_shape=True)become pretty similar in performance for single GPU tests. The main 2 differences seem to be:ImperativeBulkis a temporaryThreadedEngineOprdestroyed at the end of its invocation, which seems to be a relatively expensive operation (hybridized model with everything static mostly does not useImperativeBulkso does not pay the cost of destruction of the object).@eric-haibin-lin @piiswrong